Skip to content

[proto] Small optim for perspective op on images#6907

Merged
vfdev-5 merged 7 commits intopytorch:mainfrom
vfdev-5:proto-update-perspective-image-2
Nov 4, 2022
Merged

[proto] Small optim for perspective op on images#6907
vfdev-5 merged 7 commits intopytorch:mainfrom
vfdev-5:proto-update-perspective-image-2

Conversation

@vfdev-5
Copy link
Contributor

@vfdev-5 vfdev-5 commented Nov 4, 2022 •
edited by pytorch-bot bot
Loading

  • Small optim for perspective op on images
  • reverted concat+single matmul trick on bboxes as it does not give any speed up and brings a slowdown for images.
[----------------------------------- Perspective_image_tensor cpu ----------------------------------]
                                   |  perspective_image_tensor_old v2  |  perspective_image_tensor v2
1 threads: ------------------------------------------------------------------------------------------
      (3, 400, 500) torch.uint8    |                4.8                |              4.64           
      (3, 400, 500) torch.float32  |                4.2                |              4.14           
6 threads: ------------------------------------------------------------------------------------------
      (3, 400, 500) torch.uint8    |                3.7                |              3.67           
      (3, 400, 500) torch.float32  |                3.6                |              3.52           

Times are in milliseconds (ms).

[---------------------------------- Perspective_image_tensor cuda ----------------------------------]
                                   |  perspective_image_tensor_old v2  |  perspective_image_tensor v2
1 threads: ------------------------------------------------------------------------------------------
      (3, 400, 500) torch.uint8    |                633                |              585            
      (3, 400, 500) torch.float32  |                549                |              543            
6 threads: ------------------------------------------------------------------------------------------
      (3, 400, 500) torch.uint8    |                594                |              586            
      (3, 400, 500) torch.float32  |                551                |              545            

Times are in microseconds (us).

cc @datumbox @bjuncek @pmeier

@vfdev-5 vfdev-5 added module: transforms Perf For performance improvements prototype labels Nov 4, 2022
datumbox
datumbox reviewed Nov 4, 2022
View reviewed changes
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @vfdev-5 just 2 questions. Looks good overall.

torchvision/prototype/transforms/functional/_geometry.py Outdated
Copy link
Contributor

@datumbox datumbox Nov 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do in-place division?

torchvision/prototype/transforms/functional/_geometry.py
Comment on lines +1053 to +1054
numer_points = torch.matmul(points, theta1.T)
denom_points = torch.matmul(points, theta2.T)
Copy link
Contributor

@datumbox datumbox Nov 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to understand, previously when we measured this specific change for bboxes it didn't improve the speed?

Copy link
Contributor Author

@vfdev-5 vfdev-5 Nov 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I measured all changes together: concat+single matmul + inplace + aminmax

Copy link
Contributor

@pmeier pmeier Nov 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, it improved the speed for bounding boxes, but the PR didn't measure the impact on images. Since bounding box tensors are usually a lot smaller than images, the perf regression for images was not noticed there.

Copy link
Contributor Author

@vfdev-5 vfdev-5 Nov 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were no perf regression at all as it was using previous implementation. For bboxes shape=(1000, 4) "concat+single matmul" only trick does not bring any speed up on cpu. While working on images (this PR) I see that same trick brings even a slowdown.

Copy link
Contributor

@datumbox datumbox Nov 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK if you measured it now and you know it doesn't slow us down it's fine by me.

datumbox
datumbox approved these changes Nov 4, 2022
View reviewed changes
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, feel free to merge after testing the in-place division.

datumbox
datumbox reviewed Nov 4, 2022
View reviewed changes
@vfdev-5 vfdev-5 merged commit 3d10c8a into pytorch:main Nov 4, 2022
@datumbox datumbox mentioned this pull request Nov 8, 2022
Closed
31 tasks
facebook-github-bot pushed a commit that referenced this pull request Nov 14, 2022
@facebook-github-bot
[fbsync] [proto] Small optim for perspective op on images (#6907)
e55c02e 
Summary:
* [proto] small optim for perspective op on images, reverted concat trick on bboxes

* revert unrelated changes

* PR review updates

* PR review change

Reviewed By: NicolasHug

Differential Revision: D41265184

fbshipit-source-id: 12073a164180b2ed392dd455106f6411bab9a317
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@datumbox datumbox datumbox approved these changes

@pmeier pmeier pmeier left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla signed module: transforms Perf For performance improvements prototype

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vfdev-5 @datumbox @pmeier @facebook-github-bot

Comments